Skip to content

DrawImage: stop re-proving what a repeat already proved, and keep rowOpaque out of line - #13

Merged
tannevaled merged 1 commit into
mainfrom
fix/image-rowopaque-inline
Aug 11, 2026
Merged

DrawImage: stop re-proving what a repeat already proved, and keep rowOpaque out of line#13
tannevaled merged 1 commit into
mainfrom
fix/image-rowopaque-inline

Conversation

@tannevaled

Copy link
Copy Markdown
Contributor

Two defects, both found by measuring the widgets rather than the primitive, and both invisible from reading the code.

v0.7.0 regressed the cases it was not meant to touch

A full 1000×700 blit went from 265,039 ns/op at v0.5.0 to 1,152,535. The clipped rewrite made the loop bigger, and rowOpaque — a tight strided scan whose cost lives entirely in register allocation — got inlined into it. Kept out of line, the same code measures 238,252.

The //go:noinline directive carries that pair of numbers in its comment, because whoever deletes it deserves to know what it costs.

Worth recording how this was found: pprof blamed rowOpaque while quietly attributing memmove's work to it, and the bounds-check dump was identical between versions. Neither told the truth. What settled it was crossing the two implementations against each other on the same machine back to back, then crossing the two inlining choices — a control run, not a reading of the profile.

An enlarged blit scanned every source row twice

Once when the row was built, once again for each destination row repeating it: 700 scans of 1.4 MB per blit to prove 350 rows opaque. Settling the repeat before touching the source removes that half outright. prevSY only ever holds a row that was written whole, so a match already proves opacity — there is nothing to re-check.

Result

shape DrawImage per-pixel
1:1 279,726 1,867,584 6.7×
enlarged 351,219 2,066,103 5.9×
clipped 314,708 2,644,680 8.4×

Every shape is now faster than at any earlier version — v0.6.0 blit scaled in 434,038 and v0.7.0 blit clipped in 911,278.

100% statement coverage. The 16-case comparison against the loop DrawImage replaced is unchanged and still passes.

🤖 Generated with Claude Code

…Opaque out of line

Two defects, both found by measuring the widgets rather than the
primitive, and both invisible from reading the code.

v0.7.0 regressed the unclipped cases it was not meant to touch: a full
1000x700 blit went from 265,039 ns/op at v0.5.0 to 1,152,535. The clipped
rewrite made the loop bigger, and rowOpaque -- a tight strided scan whose
cost lives entirely in register allocation -- was inlined into it. Kept
out of line the SAME code measures 238,252. The directive carries that
pair of numbers, because a reader deleting it deserves to know what it
costs. Profiling pointed at rowOpaque while attributing memmove's work to
it; only crossing the two implementations against each other, then the
two inlining choices, settled which of the two was to blame.

Separately, an enlarged blit scanned every source row TWICE: once when
the row was built, once for each destination row repeating it -- 700
scans of 1.4 MB per blit to prove 350 rows opaque. Settling the repeat
before touching the source removes that half outright. prevSY only ever
holds a row that was written whole, so a match already proves opacity.

  1:1        279726 ns/op  (per-pixel 1867584)   6.7x
  enlarged   351219 ns/op  (per-pixel 2066103)   5.9x
  clipped    314708 ns/op  (per-pixel 2644680)   8.4x

Every shape is now faster than it was at any earlier version: v0.6.0 blit
scaled in 434,038 and v0.7.0 blit clipped in 911,278.

100% statement coverage; the 16-case comparison against the loop
DrawImage replaced is unchanged and still passes.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@tannevaled
tannevaled merged commit 2be1ce9 into main Aug 11, 2026
1 check passed
@tannevaled
tannevaled deleted the fix/image-rowopaque-inline branch August 11, 2026 09:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant